Repository navigation
Conversation
|
Looks good to me.
I think it would be interesting to get performance information when the connection goes over the internet. Compression would be more benificial on slow links, but I guess it would not make much of a difference as long as the query fits in one packet. |
|
I am in favor of this change and it makes sense to me. But I would like to see some justification for 512. Why is it 512 and not 50? Or maybe something in the middle? How much compression can we get by compressing a 512-byte-long string? Why is it even a power of 2? I would rather keep it at 50, just like libmysqlclient does or at most increase it to 100 bytes. Regarding the new test: it just looks like AI slop. I don't understand what exactly it tests or its relation to the change in this PR. It doesn't seem to verify that compression is happening. I think you should write the test yourself, or, if there is one, adjust an existing test to cover the branch change. |
|
There is no good reason for 512, and it being a power of 2 means nothing. I measured it on single statements of the kinds PHP applications send. I used 200 of each with random values and compressed them with
|
Yes, Claude gave me an almost identical table. From it, I read that 100 is the most that we can apply because after 100 bytes we already start to see some significant compression. Claude does not seem to understand that we are not trying to optimise the performance of the local CPU but the network instead. I think that keeping it at 50 bytes is the safest option. |
A new test of 8,665 packets from 0 to 700B shows packets of 50–99 B saving 1.7 B on average (median 0). From 100–149 B they save 9% and from 150 B up to 18-28%. So any threshold less 100 makes little sense. |
|
More bytes do not necessarily mean less performance. My understanding is that it makes little difference as long as it fits in a single network packet. Is performance the only consideration here? I can imagine users that pay per byte for their bandwidth and want to enable compression to reduce bandwidth costs? |
|
We had a case: a high-throughput service whose link to the database was at 1 Gbit, and we had to turn compression on to fit the traffic. But at thousands of requests per second the client CPU matters too, so every compress() call that saves nothing on the wire is pure cost. |
|
In that case, let's settle on 100 bytes. |
Below 100 bytes compress() saves 1.7 bytes per packet on average, so the call is not worth it there. The mysqli compression tests pass.
With
MYSQLI_CLIENT_COMPRESS, mysqlnd runscompress()on every packet it sends, however small. Short packets do not shrink, so for them the call is pure cost for the client.This sends packets smaller than 100 bytes uncompressed inside the compressed envelope (uncompressed length 0), which the protocol already allows and which mysqlnd already does for packets that do not shrink. libmysqlclient uses the same mechanism with a 50 byte threshold.
I compressed 8,665 statements of the kinds PHP applications send (ORM selects, inserts, updates, prepared statement executes, WordPress and session queries), from a few bytes to 700, with
compress()at level 6. Packets under 50 bytes do not shrink. Packets of 50 to 99 bytes save 1.7 bytes on average, with a median of 0. From 100 to 149 bytes they save about 9%, and from 150 bytes 18% to 28%. So below 100 bytes the call saves next to nothing on the wire.The test uses the fake server to read what the client sends and checks the uncompressed length in the compressed header: 0 for a 94 byte query and the real length for a 1014 byte one. Without the change the 94 byte query is sent compressed and the test fails.